Reduce memory consumption of two ranges tests #2657

CaseyCarter · 2022-04-16T02:31:37Z

By making the stateful lambda a namespace-scope function object (at the suggestion of the FE team) and reducing the combinatorics of the test matrix of range properties.

miscco

Thanks a lot, i started to also rely more on struct these days

AlexGuteniev · 2022-04-20T08:50:04Z

and reducing the combinatorics of the test matrix

Where? 👀

barcharcraz

How much time/mem does the struct conversion alone save? Without running only "interesting" specializations?

tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp

StephanTLavavej · 2022-04-21T22:48:23Z

I have pushed a trivial change to remove unnecessary test:: qualification, which looks more complicated than it is due to (desirable) clang-format reflowing.

StephanTLavavej · 2022-04-21T22:51:19Z

@CaseyCarter

and reducing the combinatorics of the test matrix

@AlexGuteniev

Where? 👀

I believe that this is happening, not in the meow_matrix.lst, but in replacing the runtime call to test_in_write<instantiator, const P, P>(); (now guarded by TEST_EVERYTHING) with a few specific lines of instantiator::call.

CaseyCarter · 2022-04-21T23:00:18Z

and reducing the combinatorics of the test matrix

Where? 👀

The body of main. The previous body is still there under #ifdef TEST_EVERYTHING. test_in_write:

STL/tests/std/include/range_algorithm_support.hpp

Lines 1295 to 1298 in 0349ce1

    
           template <class Instantiator, class Element1, class Element2> 
        
           constexpr void test_in_write() { 
        
               with_input_ranges<with_writable_iterators<Instantiator, Element2>, Element1>::call(); 
        
           }

generates calls to instantiator::call with the cross product of "all variations of input range properties" (about 55 cases):

STL/tests/std/include/range_algorithm_support.hpp

Lines 1047 to 1086 in 0349ce1

    
           // For all ranges, IsCommon implies Eq. 
        
           // For single-pass ranges, Eq is uninteresting without IsCommon (there's only one valid iterator 
        
           // value at a time, and no reason to compare it with itself for equality). 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::no, Common::no, CanCompare::no, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::no, Common::no, CanCompare::no, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::no, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::no, Common::no, CanCompare::no, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::no, Common::no, CanCompare::no, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::no, Common::yes, CanCompare::yes, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::yes, Common::no, CanCompare::no, ProxyRef::yes>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               range<input, Element, Sized::yes, CanDifference::yes, Common::yes, CanCompare::yes, ProxyRef::yes>>(); 
        
           with_forward_ranges<Continuation, Element>::template call<Args...>();

and "all variations of writable iterator properties" (about 15 cases):

STL/tests/std/include/range_algorithm_support.hpp

Lines 898 to 904 in 0349ce1

    
           // Diff and Eq are not significant for "lone" single-pass iterators, so we can ignore them here. 
        
           Continuation::template call<Args..., 
        
               iterator<input, Element, CanDifference::no, CanCompare::no, ProxyRef::no>>(); 
        
           Continuation::template call<Args..., 
        
               iterator<input, Element, CanDifference::no, CanCompare::no, ProxyRef::yes>>(); 
        
           with_output_iterators<Continuation, Element>::template call<Args...>();

for a total of 15 * 55 = 825 specializations tested. This is replaced by directly calling instantiator::call five different ways. We know this is adequate coverage only by knowing the details of the implementation.

CaseyCarter · 2022-04-21T23:09:41Z

How much time/mem does the struct conversion alone save?

I only measured remove_copy. (@xiangfan-ms reported similar numbers for remove_copy_if which was motivation enough for me to add it without measuring myself.) Only this change affects the maximum memory size, which went from 5.8GB to 125MB. The corresponding reduction in runtime for all 17 test configs on my 3950X was (IIRC) 90 seconds down to the low twenties.

Without running only "interesting" specializations?

This further reduced total runtime down to about 2.5 seconds.

StephanTLavavej · 2022-04-26T21:26:17Z

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

StephanTLavavej · 2022-04-27T02:00:21Z

Thanks for dramatically improving the resource consumption of these tests! 🐱 🎉 📉

Osyotr · 2022-04-27T08:34:58Z

How could this lambda cause this extensive memory consumption?

miscco · 2022-04-27T12:12:41Z

How could this lambda cause this extensive memory consumption?

Every time when we instantiate the function, we create a new stateful lambda. And we instantiate that function a ton

Reduce ranges_alg_remove_copy memory

99aa63d

By making the stateful lambda a namespace-scope function object (at the suggestion of the FE team) and reducing the combinatorics of the test matrix of range properties.

CaseyCarter added test Related to test code ranges C++20/23 ranges labels Apr 16, 2022

CaseyCarter added this to Initial Review in Code Reviews via automation Apr 16, 2022

CaseyCarter requested a review from a team as a code owner April 16, 2022 02:31

miscco approved these changes Apr 16, 2022

View reviewed changes

Apply the same transformations to P0896R4_ranges_alg_remove_copy_if

e8d8ef8

CaseyCarter moved this from Initial Review to Final Review in Code Reviews Apr 19, 2022

CaseyCarter changed the title ~~Reduce ranges_alg_remove_copy memory~~ Reduce memory consumption of two ranges tests Apr 19, 2022

StephanTLavavej assigned barcharcraz Apr 20, 2022

barcharcraz added this to @barcharcraz in Maintainer Priorities Apr 20, 2022

barcharcraz approved these changes Apr 21, 2022

View reviewed changes

tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp Show resolved Hide resolved

raghavthind2005 approved these changes Apr 21, 2022

View reviewed changes

StephanTLavavej unassigned barcharcraz Apr 21, 2022

StephanTLavavej reviewed Apr 21, 2022

View reviewed changes

tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp Outdated Show resolved Hide resolved

tests/std/tests/P0896R4_ranges_alg_remove_copy/test.cpp Show resolved Hide resolved

Using-declarations: my favorite nitpick!

5582268

StephanTLavavej approved these changes Apr 21, 2022

View reviewed changes

StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews Apr 21, 2022

StephanTLavavej self-assigned this Apr 26, 2022

StephanTLavavej merged commit 314f65f into microsoft:main Apr 27, 2022

Code Reviews automation moved this from Ready To Merge to Done Apr 27, 2022

Maintainer Priorities automation moved this from @barcharcraz to Done Apr 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce memory consumption of two ranges tests #2657

Reduce memory consumption of two ranges tests #2657

CaseyCarter commented Apr 16, 2022

miscco left a comment

AlexGuteniev commented Apr 20, 2022

barcharcraz left a comment

StephanTLavavej commented Apr 21, 2022

StephanTLavavej commented Apr 21, 2022

CaseyCarter commented Apr 21, 2022 •

edited

CaseyCarter commented Apr 21, 2022 •

edited

StephanTLavavej commented Apr 26, 2022

StephanTLavavej commented Apr 27, 2022

Osyotr commented Apr 27, 2022

miscco commented Apr 27, 2022

Reduce memory consumption of two ranges tests #2657

Reduce memory consumption of two ranges tests #2657

Conversation

CaseyCarter commented Apr 16, 2022

miscco left a comment

Choose a reason for hiding this comment

AlexGuteniev commented Apr 20, 2022

barcharcraz left a comment

Choose a reason for hiding this comment

StephanTLavavej commented Apr 21, 2022

StephanTLavavej commented Apr 21, 2022

CaseyCarter commented Apr 21, 2022 • edited

CaseyCarter commented Apr 21, 2022 • edited

StephanTLavavej commented Apr 26, 2022

StephanTLavavej commented Apr 27, 2022

Osyotr commented Apr 27, 2022

miscco commented Apr 27, 2022

CaseyCarter commented Apr 21, 2022 •

edited

CaseyCarter commented Apr 21, 2022 •

edited